Build opus with NEON intrinsics on ARM (Android ABIs included) by mcfnord · Pull Request #3833 · jamulussoftware/jamulus

mcfnord · 2026-07-24T03:36:40Z

🤖 AI: Fixes #2806.

Jamulus.pro's opus ARM block matched only the Android ABI names (armeabi-v7a, arm64-v8a), so every other 64-bit ARM build (Apple Silicon macOS, iOS, Linux aarch64, which Qt reports as QT_ARCH=arm64) compiled opus as plain C. The block was also inert where it did match: SOURCES += $$SOURCES_OPUS_ARCH existed only in the x86 branch, and OPUS_ARM_MAY_HAVE_NEON_INTR, which gates opus's arm/*.h headers, was never defined. On main no target compiles a single libs/opus/*/arm/ file.

What this PR does

One ARM case for all three values of QT_ARCH: same headers, the 9 NEON intrinsic sources, OPUS_ARM_MAY_HAVE_NEON_INTR=1 OPUS_ARM_PRESUME_NEON=1 OPUS_ARM_PRESUME_NEON_INTR=1, plus OPUS_ARM_PRESUME_AARCH64_NEON_INTR for the 64-bit ones.
A consumption branch that compiles those sources for the ARM cases. No special compiler flags: NEON is part of the base AArch64 ISA, and the NDK's clang enables it by default for armv7a-linux-androideabi.
celt_fft_ne10.c and celt_mdct_ne10.c removed from the source list. They #include <NE10_dsp.h> unconditionally, Jamulus does not bundle Ne10, and no target has ever compiled them.

The Android ABIs therefore compile the NEON sources for the first time; x86 builds are untouched.

Testing

qmake-resolved Makefile.Release per QT_ARCH: x86_64 identical before and after; arm64, arm64-v8a, armeabi-v7a each list the 9 ARM sources and the defines above; main lists 0 ARM sources for the Android ABIs.
Native Raspberry Pi 4 build (aarch64, gcc 14, CONFIG+=headless): links, 9 _neon symbols in the binary, 0 undefined.
NDK r21d clang with Qt 5.15.2's android-clang flags (-target <abi>30 -Oz -fPIC): all 138 opus sources compile for armv7a-linux-androideabi30 and aarch64-linux-android30, 0 failures, and every _neon symbol the objects reference is defined in the set.
CPU effect on a Pi 4 server was measured earlier in this thread as noise-level at both complexity settings; this is a build-correctness fix, not a performance claim.

🤖 This message was written by AI and reviewed by @mcfnord.

…tware#2806) The opus arch-optimisation block only matched the Android ABI names `armeabi-v7a` and `arm64-v8a`. Everywhere else Qt reports 64-bit ARM as `QT_ARCH=arm64` (Apple Silicon macOS, iOS, Linux aarch64), so those builds fell through to the plain-C opus path with no NEON acceleration. In fact the ARM NEON sources were never compiled on any target: the `SOURCES += $$SOURCES_OPUS_ARCH` line only lived in the x86 branch, so even the Android arch match populated the variable but never built it. Add an `arm64` branch that: - defines OPUS_ARM_MAY_HAVE_NEON_INTR (required for opus to include its arm/*.h headers) plus the PRESUME_NEON_INTR / PRESUME_AARCH64_NEON_INTR defines (NEON is part of the base AArch64 ISA, so it is always present); - compiles the NEON intrinsic sources, which need no special compiler flags, straight into SOURCES. Split SOURCES_OPUS_ARM into the self-contained NEON intrinsic files and the two NE10 files, which #include <NE10_dsp.h> from the external Ne10 library that Jamulus does not bundle; only the NEON subset is compiled. The x86 and Android arch matches are untouched: their generated Makefiles are byte-identical before and after this change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

ann0see · 2026-07-25T17:14:57Z

Should have Autobuild please build all targets

ann0see · 2026-07-25T17:15:33Z

    libs/opus/src/repacketizer.c

-SOURCES_OPUS_ARM = libs/opus/celt/arm/armcpu.c \
+# NEON intrinsic sources: self-contained, need no external library.


Suggested change

# NEON intrinsic sources: self-contained, need no external library.

🤖 AI: All three done in cbe7d44a. The comment line is gone. The NE10 pair is dropped: both files #include <NE10_dsp.h> unconditionally and Jamulus does not bundle Ne10, so no target has ever compiled them. The arm64 case is merged into the Android one, and the merge has a consequence: on main the Android ABIs define the PRESUME macros but compile zero libs/opus/*/arm/ files, because the SOURCES consumption branch covers only x86. Now all three ARM values of QT_ARCH compile the nine NEON files. Checked: the x86_64 Makefile is unchanged; a native Pi 4 build links the same nine _neon symbols as before; NDK r21d clang with Qt 5.15.2's flags compiles all 138 opus files for both Android ABIs, NEON being its default for armv7a-linux-androideabi. The Android CI job is green on this head.

CodeRabbit's summary took 142 lines to report that it had nothing actionable to say; a review with nothing to say has one right length.

ann0see · 2026-07-25T19:39:04Z

Also this needs to be built, tested and analyzed on Apple Silicon. The arm handling in general is sketchy. For macOS Apple Silicon the explicit variable setting of arm64 is needed otherwise it tries to compile SSE for arm64 which obviously doesn't work

mcfnord · 2026-07-26T03:25:40Z

Note

📡 STAND BY FOR AN LLM-AUTHORED MESSAGE.

Native verification on a Raspberry Pi 4 (Cortex-A72, aarch64 Linux, not cross-compiled), including a result I didn't expect.

Symbols: built this branch and the same base commit without it, both natively. nm on the shipped binary confirms all 9 *_neon functions defined in the fixed build, 0 in the unfixed one — the fix links and ships correctly.

CPU benefit: interleaved A/B benchmark (server pinned to one core, bot load, perf stat task-clock), at OPUS_SET_COMPLEXITY(1) (Jamulus's actual setting) I measured no significant difference — and re-tested at OPUS_SET_COMPLEXITY(10) in case low complexity was masking a win: still no significant difference.

Why: confirmed with perf record -g under load — Jamulus uses opus_custom_* (Opus Custom, CELT-only; SILK never runs). 6 of the 9 accelerated functions are SILK-only and are dead code in Jamulus regardless of this fix. Of the 3 CELT functions, only 2 ever appeared under profiling, combined under 1% of server CPU.

Bottom line: still worth merging — dead ARM code silently no-op'ing is a real defect on its own terms — but I'd frame it as a correctness/build-hygiene fix rather than a proven Jamulus performance win. This only covers Linux aarch64; it doesn't address the Apple Silicon question above.

pljones · 2026-07-26T13:36:10Z

I'd love to get my local (Ubuntu 24.04 LTS) Android build working. Currently I rely on Github.

pljones · 2026-07-26T13:37:23Z

#2806 is "mac" -- this is "android". Is #2806 mis-titled?

mcfnord · 2026-07-27T06:59:35Z

Note

📡 STAND BY FOR AN LLM-AUTHORED MESSAGE.

Catching up on the thread:

@ann0see, re: build all targets — CI already did, on this PR's own run: Android APK, Linux arm64 .deb, armhf .deb, macOS, macOS Legacy, iOS, Windows, Windows JACK all pass (https://github.com/jamulussoftware/jamulus/actions/runs/30064659704). The macOS job builds a universal binary and mac/deploy_mac.sh explicitly sets QT_ARCH=arm64 for the Apple Silicon slice, so that slice does compile clean with this fix. I don't have Apple hardware, so that's a compile proof only — no perf number for Apple Silicon from me, same caveat as my Pi 4 result below.

@pljones, re: is #2806 mis-titled — fair catch, and a bit of both: #2806's title says "Mac ARM," which matches how the issue was originally reported, but this PR's actual scope is wider than that title suggests. It touches every non-Android 64-bit ARM target that reports QT_ARCH=arm64 (macOS Apple Silicon, iOS, Linux aarch64) — and separately, while implementing that, it surfaced a genuine Android-specific bug: Android's own arch match populated the NEON source list but two other gaps meant those sources were never actually compiled or linked, so Android has been silently running plain-C opus too, this whole time. That second part isn't really "about" #2806 at all — it's its own defect that happened to surface in the same investigation.

My conclusion, for the record: native Pi 4 (Cortex-A72, Linux aarch64) testing confirms the fix is real and correct — all 9 NEON symbols ship where 0 did before — but produces no measurable CPU win for Jamulus specifically, at either complexity=1 (Jamulus's actual setting) or complexity=10 (max), because Jamulus's CELT-only Opus Custom mode only ever touches 2 of those 9 functions, combined under 1% of server CPU. That's a Jamulus-specific result, not a NEON-doesn't-help result — other opus consumers using SILK or the general encoder path would very plausibly see more. I'd still call this worth merging on correctness/build-hygiene grounds (dead ARM code silently no-op'ing across four platforms is a real defect on its own), just not on a proven performance claim. That's everything I can verify from here — Apple Silicon and a from-scratch Android build both remain open if anyone has the hardware.

ann0see

...

ann0see

Please clean up the comments as mentioned above.

Review follow-up on jamulussoftware#3833: - The arm64 case is merged into the existing Android case. All three ARM values of QT_ARCH (armeabi-v7a, arm64-v8a, arm64) now get the same headers, sources and defines, with OPUS_ARM_PRESUME_AARCH64_NEON_INTR added for the two 64-bit ones. OPUS_ARM_MAY_HAVE_NEON_INTR is defined for all of them: opus gates the inclusion of its arm/*.h headers on it, so without it the PRESUME defines select nothing. - The SOURCES consumption branch covers the Android ABIs too. On main it covers only x86, so the Android build defines the PRESUME macros but never compiles a single libs/opus/*/arm/ file (qmake-resolved on 2026-09-05: 0 arm sources for arm64-v8a and armeabi-v7a on main). - celt_fft_ne10.c and celt_mdct_ne10.c are removed from the list. They #include <NE10_dsp.h> unconditionally, Jamulus does not bundle Ne10, and no target has ever compiled them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017veM89EdPrzUbWbb44aKQs

coderabbitai · 2026-09-05T19:46:55Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: QUIET

Plan: Advanced

Run ID: 5c186f4e-07a9-4706-9528-8635208519cb

📥 Commits

Reviewing files that changed from the base of the PR and between 9779918 and cbe7d44.

📒 Files selected for processing (1)

Jamulus.pro

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The ARM Opus build configuration now supports Android ARM ABIs and plain arm64 targets. It enables the required NEON definitions, removes NE10 sources, and compiles ARM architecture sources directly.

Changes

ARM Opus build targeting

Layer / File(s)	Summary
Architecture selection and compilation `Jamulus.pro`	ARM Opus detection and NEON defines now include `arm64`. The NE10 FFT and MDCT sources were removed. ARM architecture sources are compiled directly for `armeabi-v7a`, `arm64-v8a`, and `arm64`.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to cbe7d

This change enables the intended Opus ARM NEON build path for supported ARM targets while excluding unsupported NE10 sources. The updated configuration has no identified merge-blocking risk.

Suggested reviewers: ann0see

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The Android ABI source and define changes extend beyond issue [`#2806`], whose stated requirement is to enable ARM optimizations for Mac M1 builds. These changes are related to ARM optimization work but…	Remove the Android behavior changes from this PR, or link an Android-specific issue and update the scope and acceptance criteria to cover the Android source and define changes.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies issue [`#2806`] by enabling Opus ARM optimizations for non-Android arm64 targets, including Apple Silicon macOS. The implementation provides the required ARM defines and compiles the NE…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Title check	✅ Passed	The title clearly summarizes the main change: enabling Opus NEON intrinsics for ARM targets, including Android ABIs.
Description check	✅ Passed	The description provides a detailed change summary, issue context, testing results, platform scope, and documentation considerations. It does not reproduce the template headings or checklist, but the …

Full details: Out of Scope Changes check

Explanation

The Android ABI source and define changes extend beyond issue [#2806], whose stated requirement is to enable ARM optimizations for Mac M1 builds. These changes are related to ARM optimization work but are not covered by the linked issue's acceptance criteria.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

ann0see reviewed Jul 25, 2026

View reviewed changes

Comment thread Jamulus.pro Outdated

Comment thread Jamulus.pro Outdated

mcfnord mentioned this pull request Jul 26, 2026

Investigate opus build optimizations for ARM non-FP (Raspberry, Android, iOS) #2475

Open

4 tasks

pljones added the android Android runtime issue label Jul 26, 2026

pljones added this to Tracking Jul 26, 2026

github-project-automation Bot moved this to Triage in Tracking Jul 26, 2026

dingodoppelt added the AI AI generated or potentially AI generated label Jul 28, 2026

ann0see reviewed Aug 2, 2026

View reviewed changes

ann0see reviewed Aug 13, 2026

View reviewed changes

ann0see moved this from Triage to Waiting externally in Tracking Aug 15, 2026

ann0see added this to the Release 4.0.0 milestone Sep 1, 2026

mcfnord changed the title ~~Build opus with NEON intrinsics on non-Android 64-bit ARM~~ Build opus with NEON intrinsics on ARM (Android ABIs included) Sep 5, 2026

Uh oh!

Conversation

mcfnord commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ann0see commented Jul 25, 2026

Uh oh!

ann0see Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

ann0see Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

mcfnord Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ann0see commented Jul 25, 2026

Uh oh!

Uh oh!

Uh oh!

mcfnord commented Jul 26, 2026

Uh oh!

pljones commented Jul 26, 2026

Uh oh!

pljones commented Jul 26, 2026

Uh oh!

mcfnord commented Jul 27, 2026

Uh oh!

ann0see left a comment

Choose a reason for hiding this comment

Uh oh!

ann0see left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mcfnord commented Jul 24, 2026 •

edited

Loading

mcfnord Sep 5, 2026 •

edited

Loading

coderabbitai Bot commented Sep 5, 2026 •

edited

Loading